-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Exclude RDS objects and rdsadmin DB from sp_Blitz checks #3713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Exclude RDS objects and rdsadmin DB from sp_Blitz checks #3713
Conversation
BrentOzar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Thanks for the pull request, but it looks like maybe some unrelated changes snuck in? Or can you explain these other changes, not sure why they were made? Sorry about the delay.
| IF @Debug IN (1, 2) RAISERROR('Running CheckId [%d].', 0, 1, 99) WITH NOWAIT; | ||
|
|
||
| EXEC dbo.sp_MSforeachdb 'USE [?]; SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED; IF EXISTS (SELECT * FROM sys.tables WITH (NOLOCK) WHERE name = ''sysmergepublications'' ) IF EXISTS ( SELECT * FROM sysmergepublications WITH (NOLOCK) WHERE retention = 0) INSERT INTO #BlitzResults (CheckID, DatabaseName, Priority, FindingsGroup, Finding, URL, Details) SELECT DISTINCT 99, DB_NAME(), 110, ''Performance'', ''Infinite merge replication metadata retention period'', ''https://www.brentozar.com/go/merge'', (''The ['' + DB_NAME() + ''] database has merge replication metadata retention period set to infinite - this can be the case of significant performance issues.'')'; | ||
| EXEC dbo.sp_MSforeachdb 'USE [?]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we change this section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BrentOzar I just added a new line for clarity.
| GROUP BY LEFT(physical_name, 1) | ||
| HAVING COUNT(*) > 1 | ||
| AND SUM(size) < 268435456 OPTION (RECOMPILE);'; | ||
| HAVING COUNT(*) > 1 AND SUM(size) < 268435456 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we change this section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BrentOzar I just reformatted the lines to separate the GROUP BY from the OPTION part for clarity.
|
I haven't heard back in a couple of weeks, so I'm going to go ahead and close this for now. If you'd like to resubmit it with only the relevant changes, though, we'd welcome it! Thanks. |
|
Ping @BrentOzar |
|
@yoni-yad2 yes, just busy - can you resubmit a pull request with only the relevant changes? I know it sounds dumb, but it's volunteers here on the other side of the screen, and I need to be careful with what we have to test. Hope that's fair. |
#3712